mz873: hash composite cache keys recursively#875
Draft
mzihlmann wants to merge 2 commits into
Draft
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
31db7b2 to
e460eb5
Compare
e460eb5 to
ae136b3
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #873
The composite cache key was the hash of all key parts joined with '-'. Because '-' is legal inside the parts, distinct part sequences can join to the same string and collide, at which point one build serves another build's cached layer. The collision does not even need two builds: Dockerfile_test_issue_mz873 makes two sibling stages collide within a single build. Stage one's single RUN spells out the joined key text of stage two's two RUNs, with a '#' turning the spelled-out tail into a shell comment. On main both commands key at the same value, both push their layer to the same tag, and the warm build serves the swapped layers, /one and /two silently exchange content between the cold and the warm build.
Behind the new FF_KANIKO_ROLLING_CACHE_KEY flag the composite cache folds every part into a fixed-size rolling state, state' = SHA256(state || part), and the state is the canonical key. The state has a fixed length, so the boundary to the next part is unambiguous and the collision class is gone. A chain prefix becomes a resumable state, which FROM-child seeding and the redirect pointer use, so the pointer no longer serializes the unbounded key text into an image label. The human readable key trail is kept for the cache-miss debug logs only. The flag defaults to off and off is byte-compatible with today's keys, switching it changes every cache key and rebuilds once, documented in the README.
The standard TestCache run of Dockerfile_test_issue_mz873 pins the fix: on main it fails with a content mismatch between the cold and the warm build, with the flag it passes. The integration suite and the golden suite run with the flag on.